Skip to content

feat: Enable CSP with nonce for Helix 5 - 2nd try #816

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 10 commits into from
Feb 12, 2025
Merged

Conversation

andreituicu
Copy link
Collaborator

1. Description

  1. Allow customers to define a nonce based CSP using either a meta tag or header, with cached nonce, with the keyword 'nonce-aem'.

Meta:

    <meta http-equiv="content-security-policy" content="script-src 'nonce-aem' 'strict-dynamic';  base-uri 'self'; objectsrc 'none';">

Header

content-security-policy: script-src 'nonce-aem' 'strict-dynamic'; base-uri 'self'; object-src 'none';
  1. Developers need to also add nonce="aem" on:
  • each script from head.html
  • each script from static html files in github

Example:

<script nonce="aem" src="/scripts/aem.js" type="module"></script>
<script nonce="aem" src="/scripts/scripts.js" type="module"></script>
  1. Add support for a move-as-header attribute for the meta based CSP, where the Helix Pipeline will that tag as a header

Note: if this feature is premature, I can remove it and file it as a separate PR

Example:

<meta http-equiv="content-security-policy" content="script-src 'nonce-aem' 'strict-dynamic'; base-uri 'self'; object-src 'none';" move-as-header="true">
  1. The pipeline will generate a 16byte base64 encoded nonce, place it the aem part of the nonce-aem CSP and on each trusted script where it finds nonce="aem"

2. Implementation choice

After some back and forth between:

  • having 'nonce' vs 'nonce-aem' as a keyword in the CSP
  • requiring developers to add 'nonce="aem" or not to the scripts, to make it as friction-less and transparent as possible for devs.

I chose for the moment 'nonce-aem' and needing to add 'nonce="aem" to the scripts, because in this implementation if there's any reason to deactivate/remove or if the hlx pipeline fails to render the nonce, customers that have this enabled will still have a valid page that is rendered by the browser.
In the other variants, if for any reason 'nonce' is returned the browser will show just a white page.

3. Expected level of protection with a nonce cached on the CDN

1. Protected even if the nonce is cached

.innerHTML for XSS
.outerHTML for XSS
.insertAdjecentHTML for XSS
.setHTMLUnsafe for XSS
eval / setTimeout / setInterval / Function for XSS
javascript: protocol in the href/src attributes
location.href / location.assign() / location.replace for XSS
✅ attribute event handlers (onclick, onblur, onload, onerror etc.) for XSS
✅ stored/reflected XSS

  • as long as there is no server side manipulation of the HTML in the customer's CDN where user input is involved
  • as long as the nonce is regenerated for requests that hit the origin
  • for meta tag: as long as the XSS doesn't happen before the meta tag

2. not protected, because the nonce is cached

document.write / document.writeln -> because you could get the nonce and use it in the exploit

3.not protected for other reasons

document.createRange().createContextualFragment -> whether the nonce is cached/known is irrelevant, scripts are always executed with strict-dynamic. (needs ticket in https://github.com/w3c/webappsec-csp - awaiting OSS approval)
src attribute of a <script> tag created by document.createElement('script') / text content of a <script> tag created by document.createElement('script') / import -> permitted by strict-dynamic

4. not protected by CSPs in general
Script-less attacks
❌ HTML injection
❌ DOM clobbering

5. unknown status
element.style or cssText -> still researching, can’t find a working exploit even without CSP

4. Still TODO

  • Backport to helix 4 in the 5.x branch.
  • Validate that pages render correctly E2E in the helix 4 branch (helix5 doesn't support - hlx-pipeline-version)
  • Add support in the aem cli for nonce on the live-reload script

Copy link

codecov bot commented Feb 12, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 100.00%. Comparing base (e723fe4) to head (a98a2eb).
Report is 6 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##              main      #816    +/-   ##
==========================================
  Coverage   100.00%   100.00%            
==========================================
  Files           45        46     +1     
  Lines         3671      3914   +243     
==========================================
+ Hits          3671      3914   +243     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@andreituicu andreituicu changed the title feat: Enable CSP with nonce feat: Enable CSP with nonce for Helix 5 Feb 12, 2025
@andreituicu andreituicu changed the title feat: Enable CSP with nonce for Helix 5 feat: Enable CSP with nonce for Helix 5 - 2nd try Feb 12, 2025
@andreituicu
Copy link
Collaborator Author

andreituicu commented Feb 12, 2025

Two important changes as opposed to #773, which wasn't working in the CloudFlare:

  1. Using #crypto, instead of node crypto b1d2d6b
  2. Use the lower level Tokenizer from parse5, which is already in the dependency chain, instead of bringing additional dependencies a98a2eb

adobe/helix-aem-pipeline-worker is not complaining anymore at build time.

npm install adobe/helix-html-pipeline#csp-nonce
npm run build

helix-aem-pipeline-worker tuicu$ npm run build

> helix-aem-pipeline-worker@1.1.81 build
> node build.js

bundled worker in 59 ms

@andreituicu andreituicu merged commit 3f4895f into main Feb 12, 2025
9 checks passed
@andreituicu andreituicu deleted the csp-nonce branch February 12, 2025 16:51
github-actions bot pushed a commit that referenced this pull request Feb 12, 2025
# [6.20.0](v6.19.1...v6.20.0) (2025-02-12)

### Features

* Enable CSP with nonce for Helix 5 ([#816](#816)) ([3f4895f](3f4895f))
Copy link

🎉 This PR is included in version 6.20.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants